fix(ai): Fix error propagation during setup #15379
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
FirebaseAI/Sources/Types/Internal/Live/LiveSessionService.swift
Outdated
Show resolved
Hide resolved
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the live session setup process by refactoring it into a more linear and robust async flow. The changes enhance readability and fix error propagation issues by replacing a complex Task-based mechanism with clearer, sequential helper functions. Additionally, the introduction of dedicated functions for message encoding/decoding and error mapping is a great cleanup. My review focuses on one potential edge case in the new setup logic where a graceful connection closure could leave the session in an inconsistent state, and I've provided a suggestion to make it more resilient.
I found that in certain edge case situations, errors could end up not propagating up during the initial setup of a live session. While fixing said error, I realized that the setup process could actually be cleaned up a decent amount. While this could cause issues if the model ever sent messages out of order (ie; a message before setupComplete), this doesn't seem like something we should necessarily design around. Furthermore, separating the setup process into a more iterative async style approach not only makes things easier to read (and reason about), but also solves various edge case error propagation issues.
The change has been tested on a local device, and seems to be working perfectly; existing functionality works, and the errors that were previously not being propagated are now properly bubbling up.
The process has also been split into helper functions, to make things easier to read, and to allow me to add documentation for each.
#no-changelog